Skip to content

feat(shared): Introduce client-side caching for telemetry events#3267

Closed
LauraBeatris wants to merge 8 commits intoclerk:mainfrom
LauraBeatris:add-client-cache-for-telemetry-events
Closed

feat(shared): Introduce client-side caching for telemetry events#3267
LauraBeatris wants to merge 8 commits intoclerk:mainfrom
LauraBeatris:add-client-cache-for-telemetry-events

Conversation

@LauraBeatris
Copy link
Copy Markdown
Member

@LauraBeatris LauraBeatris commented Apr 25, 2024

Description

Currently blocking SDK-1672 (#3257)

Introduces client-side caching for telemetry events. This prevents event flooding in frequently executed code paths, such as for React hooks or components.

Checklist

  • npm test runs as expected.
  • npm run build runs as expected.
  • (If applicable) JSDoc comments have been added or updated for any package exports
  • (If applicable) Documentation has been updated

Type of change

  • 🐛 Bug fix
  • 🌟 New feature
  • 🔨 Breaking change
  • 📖 Refactoring / dependency upgrade / documentation
  • other:

@LauraBeatris LauraBeatris self-assigned this Apr 25, 2024
@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Apr 25, 2024

🦋 Changeset detected

Latest commit: 43d94bc

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 11 packages
Name Type
@clerk/shared Patch
@clerk/backend Patch
@clerk/chrome-extension Patch
@clerk/clerk-js Patch
@clerk/clerk-expo Patch
@clerk/fastify Patch
@clerk/nextjs Patch
@clerk/clerk-react Patch
@clerk/remix Patch
@clerk/clerk-sdk-node Patch
gatsby-plugin-clerk Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

import type { TelemetryEventRaw } from '../types';

const EVENT_METHOD_CALLED = 'METHOD_CALLED' as const;
const EVENT_SAMPLING_RATE = 0.1;
Copy link
Copy Markdown
Member Author

@LauraBeatris LauraBeatris Apr 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also introducing a sampling rate lower than the default for this event, to handle the server-side scenarios.

CC @brkalow let me know if that makes sense

Comment thread packages/shared/src/telemetry/types.ts Outdated
@LekoArts LekoArts mentioned this pull request Apr 29, 2024
9 tasks
@panteliselef
Copy link
Copy Markdown
Contributor

This PR also enables #3279 (adding telemetry to clerk-elements)

);
}

const hasExpired = cacheEntry && now - cacheEntry > this.#cacheTtl;
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to add a test for this scenario.

@LauraBeatris LauraBeatris marked this pull request as ready for review April 29, 2024 16:19
@LauraBeatris
Copy link
Copy Markdown
Member Author

Here's is how the cache is going to look like from the local storage - a map between unit event keys and their TTL in milliseconds:

CleanShot 2024-04-29 at 13 02 13

@LauraBeatris
Copy link
Copy Markdown
Member Author

Re-opening it on #3287, this branch is from a forked repo so I think this is the cause of not allowing to run integration tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants